fix(preview): name hidden, disabled, and ambiguous click failures - #7301
fix(preview): name hidden, disabled, and ambiguous click failures#7301gbarros-dev wants to merge 8 commits into
Conversation
github-actions
Bot
added
the
vouch:trusted
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
size:L
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
One finding on the desktop-side error construction. See the inline comment.
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a focused preview-click failure-handling fix that preserves successful click behavior while carrying missing, hidden, disabled, and ambiguous outcomes through the existing IPC, web, and MCP layers. The contract changes are additive and the new paths have targeted coverage, with locator text kept out of surfaced errors. You can add or adjust custom eligibility rules. Learn more. |
bc897ad to
698bda4
Compare
Dismissing prior approval to re-evaluate 698bda4
There was a problem hiding this comment.
Reviewed the new preview automation lookup error classes against the Effect service conventions. The desktop-side classification is now a static factory next to the error type (PreviewAutomationTargetNotFoundError.fromLookupFailure), the new errors are structured Schema.TaggedErrorClass variants with messages derived from attributes, no selector text is leaked, and the host-side wrappers preserve cause. One finding: a redundant failureKind discriminator path on the web translation boundary that nothing produces.
Posted via Macroscope — Effect Service Conventions
a149228 to
b322990
Compare
Dismissing prior approval to re-evaluate b322990
5366027 to
f5145de
Compare
a46cbf3 to
def8129
Compare
def8129 to
0437863
Compare
preview_click treated a hidden or disabled button as missing and leaked the locator when it failed. Agents then spent turns guessing at chrome they could already see. Clicks now report hidden, disabled, or ambiguous targets without putting the locator on the wire.
A free function picked hidden/disabled/ambiguous and defaulted ambiguous matches to 0. The policy now lives on PreviewAutomationTargetNotFoundError.fromLookupFailure, and ambiguous requires an explicit match count.
Host mapping still accepted a failureKind field on PreviewAutomationTargetNotFoundError. Nothing emits that shape. Classification now uses the hidden, disabled, and ambiguous tags only.
0437863 to
30f017d
Compare
2 participants
preview_clickidentified hidden, disabled, and ambiguous targets in the desktop process, but Electron IPC did not preserve the custom error fields. The MCP caller then received a generic client failure.This change sends a typed click outcome across Electron IPC. The web host returns only the exact lookup reason and optional match count. The broker combines that response with the selector kind and length from the original request, so the MCP error is useful without returning the locator text.
The desktop action timeline still records target lookup failures as failed clicks.
Related to #3714. Does not close it.
Tests:
vp test run packages/contracts/src/ipc.test.tsvp test run apps/desktop/src/preview/Manager.test.ts apps/desktop/src/ipc/methods/preview.test.tsvp test run apps/web/src/components/preview/previewAutomationErrors.test.ts apps/web/src/components/preview/previewAutomationRequestConsumer.test.tsvp test run apps/server/src/mcp/PreviewAutomationBroker.test.ts apps/server/src/mcp/McpHttpServer.test.tsOriginal implementation by @gbarros-dev with Grok 4.6 through Grok CLI.
Made with GPT-5.6 Sol using Codex for modernization.
Note
Medium Risk
Changes preview automation click semantics and error propagation across desktop IPC, web host, and MCP; callers that assumed silent success on lookup failure now get typed failures, but successful clicks still dispatch input as before.
Overview
Preview automation clicks now return a typed
DesktopPreviewAutomationClickResultover Electron IPC (DispatchedorNotSentwith reasons liketarget-missing,target-hidden,target-disabled,target-ambiguous) instead ofvoid, so lookup failure details are not lost before they reach MCP callers.On the desktop side, locator resolution uses all matches and classifies missing, hidden, disabled, and ambiguous targets; failed lookups yield
NotSentwithout dispatching mouse input, while the action timeline still records failed clicks. NewPreviewAutomationTargetLookupErrortypes (contracts, desktop manager, web host) carryfailureKindand optionalmatchCount, with messages that avoid leaking locator text.The web preview host wraps IPC results with
confirmPreviewAutomationClickTarget, turning targetNotSentoutcomes intoPreviewAutomationTargetLookupHostErrorfor the automation broker/MCP path. The server broker maps remote lookup errors from responsedetail.failureKind.Reviewed by Cursor Bugbot for commit 30f017d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Return typed click outcomes for preview automation clicks
DesktopPreviewAutomationClickResultwithDispatchedorNotSentvariants (target-missing,target-hidden,target-disabled,target-ambiguous) instead ofvoidPreviewManagerlocator lookup switches from single-element to all-matches query so ambiguous targets with multiple matches are detected; only a single visible enabled match is clickedPreviewAutomationTargetLookupError,PreviewAutomationTargetLookupHostError) carry the lookup failure kind and optional positive match count, with sanitized messages that omit selector/locator valuesNotSentclick result into a named host automation error viaconfirmPreviewAutomationClickTargetDesktopPreviewBridge.automation.clickreturn type changes fromPromise<void>toPromise<DesktopPreviewAutomationClickResult | void>; callers that ignored the result are unaffected, but the broker and host layers now rejectNotSentoutcomes as errors rather than silently succeedingMacroscope summarized 30f017d.